feat: revalidation cron changed to gbfs#95
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@Alessandro100 this shouldn't be merged before #67 is completed right? |
|
yeah this shouldn't be merged before the rest of #67 is complete |
|
*Lighthouse ran on https://mobilitydatabase-2puro4qqj-mobility-data.vercel.app/ * (Desktop)
*Lighthouse ran on https://mobilitydatabase-2puro4qqj-mobility-data.vercel.app/feeds * (Desktop)
*Lighthouse ran on https://mobilitydatabase-2puro4qqj-mobility-data.vercel.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-2puro4qqj-mobility-data.vercel.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-2puro4qqj-mobility-data.vercel.app/feeds/gbfs/gbfs-flamingo_porirua * (Desktop)
|
Alessandro100
force-pushed
the
feat/67-revalidate-cron-updated
branch
from
5a74dea to
9c16c73
Compare
| AVAILABLE_LOCALES: ['en', 'fr'], | ||
| })); | ||
|
|
||
| describe('GET /api/revalidate', () => { |
There was a problem hiding this comment.
For Vercel Cron job they use GET, for external API we use POST
| }, | ||
| { | ||
| "path": "/api/revalidate", | ||
| "schedule": "0 7 * * 0" |
There was a problem hiding this comment.
[question] these two schedules are specific to gbfs feeds. is it specified anywhere?
There was a problem hiding this comment.
it's documented in src/app/api/revalidate/route.ts since we can't add comments in json
There was a problem hiding this comment.
so calling /api/revalidate with no parameters automatically revalidates all gbfs feeds?
There was a problem hiding this comment.
for the GET request which is blocked by process.env.CRON_SECRET yes
There was a problem hiding this comment.
I think the current approach couples the GBFS scope to "no params passed," which feels a bit implicit. A few concerns:
- The endpoint name
/api/revalidatereads as generic, but calling it without params actually means "revalidate all GBFS feeds", which is surprising behavior for anyone not already familiar with the route. - Documenting this in
route.tsworks for contributors reading the code, but the schedule config itself gives no indication that it's GBFS-specific. - If we ever add another feed type with its own revalidation schedule, this pattern doesn't scale cleanly.
Would it make sense to either:
- Use an explicit query param like
/api/revalidate?type=gbfs, or - Split into a dedicated route like
/api/revalidate/gbfs?
Either would make the schedules self-documenting and avoid relying on the "no params = GBFS" convention. wdyt?
There was a problem hiding this comment.
I agree that at the moment it's a little generic, adding gbfs in the path or as a param would make it much more clear when reading at a glance
2 participants
Summary:
part 1/3 of #67
Changes the revalidation cron from revalidating everything to just gbfs feeds. gtfs and gtfs_rt are going to be revalidated outside of this system
Expected behavior:
From monday to saturday at 4am utc and on sunday 7am utc we will revalidate all the gbfs feed cache. From mon-sat is the estimated time of validation finishing in GCP, and on sunday is the estimated time of geolocation finishing
Please make sure these boxes are checked before submitting your pull request - thanks!
yarn testto make sure you didn't break anything